Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tests-zt: This test stalls compiled with tests-pkg and tests-nanocoap_cache #18312

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

tests-zt: This test stalls compiled with tests-pkg and tests-nanocoap_cache #18312

wants to merge 2 commits into from

Conversation

kfessel
Copy link
Contributor

@kfessel kfessel commented Jul 15, 2022 •
edited
Loading

@miri64 found an issue with unittest and ztimer_sleep in

#17680 (comment)

Contribution description

this PR has a test to reproduce this issue

Testing procedure

make tests-zt tests-nanocoap_cache tests-pkt
make term

or

USEMODULE="ztimer_sec" make tests-zt tests-pkt

will show the issue

make tests-zt 
make term

will not

Issues/PRs references

#17680 (comment)

@kfessel kfessel requested a review from miri64 as a code owner July 15, 2022 11:00
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Jul 15, 2022
@kfessel
Copy link
Contributor Author

kfessel commented Jul 15, 2022 •
edited
Loading

i ran module info

m1.txt failing
m2.txt working

make tests-zt tests-pkt info-modules > m3
m3.txt working
USEMODULE="ztimer_sec" make tests-zt tests-pkt info-modules > m4
m4.txt failing

@kfessel kfessel added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: timers Area: timer subsystems labels Jul 15, 2022
@kfessel
Copy link
Contributor Author

kfessel commented Jul 15, 2022

USEMODULE="ztimer_sec" make tests-zt tests-pkt

also has the issue

@kfessel
Copy link
Contributor Author

kfessel commented Jul 15, 2022

USEMODULE="ztimer_sec evtimer" make tests-zt term make the issue appear

@miri64
Copy link
Member

miri64 commented Jul 15, 2022

USEMODULE="ztimer_sec evtimer" make tests-zt term make the issue appear

So maybe just removing the gnrc_ipv6 dependency from tests-pkt might be enough? Looking at the tests, this should really be gnrc_nettype_ipv6 (which was introduced later than the test was deployed).

@benpicco benpicco requested a review from kaspar030 July 15, 2022 13:22
@kaspar030
Copy link
Contributor

I think this is caused by unittests/main.c calling xtimer_init(), which in the (default) ztimer_xtimer_compat case leads to ztimer_init() being called twice. That`s not supposed to happen.

@miri64
Copy link
Member

miri64 commented Jul 15, 2022 •
edited
Loading

I think this is caused by unittests/main.c calling xtimer_init(), which in the (default) ztimer_xtimer_compat case leads to ztimer_init() being called twice. That`s not supposed to happen.

Mh... xtimer_init() used to be idempotent :-/ Could this be assured (or at least a full state reset) for ztimer_init as well?

@github-actions github-actions bot removed the Area: timers Area: timer subsystems label Jul 18, 2022
@kfessel
Copy link
Contributor Author

kfessel commented Jul 18, 2022

not calling xtimer_init when when using compat mode seems to solve

RIOT/sys/include/ztimer/xtimer_compat.h

Lines 70 to 73 in 897abd0

static inline void xtimer_init(void)
{
ztimer_init();
}

Should we remove L72? (sys/auto_init has the same do not call xtimer_init if compat mode)

@kfessel kfessel mentioned this pull request Jul 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

@kaspar030 kaspar030 Awaiting requested review from kaspar030

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Area: tests Area: tests and testing framework Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kfessel @miri64 @kaspar030